Read swapper's suggested gas limit if available#954
Conversation
|
Review changes with Changed Files
|
Summary of ChangesHello @0xh3rman, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the swap transaction process by allowing the system to dynamically adjust the gas limit based on suggestions from the swap provider, improving flexibility and potentially optimizing transaction costs. Concurrently, it introduces comprehensive support for the Okx swap provider, expanding the range of available swap services. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for reading a swapper's suggested gas limit, which is a good improvement for transaction fee accuracy. The changes are implemented for both direct Solana swaps and for proxied providers. A new provider, OKX, is also added. The implementation looks solid and is well-tested. I have one suggestion to improve the gas limit logic for proxied EVM swaps to make it more consistent.
Parse and honor provider-specified gas limits for Solana swap transactions and ensure approval-derived gas is merged with route data. - preload_mapper: for TransactionInputType::Swap read SwapData.data.gas_limit (parse as u64) and use it instead of the hardcoded 420_000 fallback. Added mock_swap_data_with_gas_limit helper and updated/added tests to cover provider gas_limit handling and to use Asset::mock_* helpers. - swapper proxy/provider: renamed approval gas var to approval_gas_limit and combine it with route data gas_limit (approval takes precedence if present) when building SwapperQuoteData. These changes allow swap providers to specify custom gas limits and ensure the final quote includes gas limits from both approval checks and route data.
0xh3rman
force-pushed
the
solana-gas-limit
branch
from
76c1740 to
7ea9596
Compare
Add support to preserve provider-specified gas_limit for non-EVM chains and update related code and tests. Introduces SwapQuoteData::mock_with_gas_limit helper, renames check_approval to check_approval_and_limit and returns the provider gas limit for non-EVM chains (previously always None). Update call site to use the new method and propagate gas_limit into SwapperQuoteData. Add MockClient and several testkit helpers plus unit tests verifying Solana preserves provider gas limits and EVM-native chains ignore them. Also add a dev-dependency on primitives with the testkit feature to Cargo.toml.
|
|
||
| [dev-dependencies] | ||
| tokio.workspace = true | ||
| primitives = { path = "../primitives", features = ["testkit"], default-features = false } |
There was a problem hiding this comment.
default-features = false should not be here
2 participants
No description provided.